Improve docs and add build() method to {Null,Boolean,}BufferBuilder
#9155
+87
−24
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alamb
commented
Jan 13, 2026
| /// | ||
| /// `NullBuffer`s can be creating using [`NullBufferBuilder`] | ||
| /// # See also | ||
| /// * [`NullBufferBuilder`] for creating `NullBuffer`s |
Contributor
Author
There was a problem hiding this comment.
adding some comments here to help navigate the maze of builders available
alamb
commented
Jan 13, 2026
| /// | ||
| /// This consumes the builder. Use [`Self::finish`] to reuse it. | ||
| #[inline] | ||
| pub fn build(self) -> BooleanBuffer { |
Contributor
Author
There was a problem hiding this comment.
most builders have a build method. The fact that the *Buffer builders in the crate do not is a small source of API friction I would like to remove
alamb
commented
Jan 13, 2026
| /// This builder only materializes the buffer when we append `false`. | ||
| /// If you only append `true`s to the builder, what you get will be | ||
| /// `None` when calling [`finish`](#method.finish). | ||
| /// This builder only materializes the buffer when null values (`false`) are |
Contributor
Author
There was a problem hiding this comment.
drive by wording cleanup
alamb
commented
Jan 13, 2026
| /// | ||
| /// # See Also | ||
| /// | ||
| /// * [`NullBuffer`] for building [`BooleanBuffer`]s for representing nulls |
Contributor
Author
There was a problem hiding this comment.
NullBufferBuilder is the correct reference so I fixed that
alamb
force-pushed
the
alamb/add_build_to_builders
branch
from
7fa4c47 to
b2e5fe4
Compare
alamb
changed the title
alamb
changed the title
{Null,Boolean,}BufferBuilder
scovich
approved these changes
Jan 13, 2026
arrow-buffer/src/builder/null.rs
Outdated
Comment on lines
212
to
213
| self.bitmap_builder | ||
| .map(|mut builder| NullBuffer::new(builder.finish())) |
Contributor
There was a problem hiding this comment.
aside: Is there an impl From that would allow this?
Suggested change
| self.bitmap_builder | |
| .map(|mut builder| NullBuffer::new(builder.finish())) | |
| self.bitmap_builder.map(Into::into) |
Jefffrey
approved these changes
Jan 14, 2026
Co-authored-by: Ed Seidl <[email protected]>
Contributor
Author
|
Thanks again everyone |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
ArrayData(1% improvement) #9120Rationale for this change
I am trying to encourage people to avoid using ArrayData when constructing arrays (as it is slower than just creating the arrays directly). Part of doing so is ensuring that the APIs to create the necessary pieces (NullBuffers in particular) are easy to use / well documented.
As pointed out by @scovich on #9120 (comment), it is
finishworks (resets the builder)buildmethod (when there is a From impl)Thus, let's add
buildmethods toNullBufferBuilderand document the difference betweenfinishandbuildWhile I was working on this change, I noticed the same issue with
BufferBuilderandBooleanBufferBuilderso I also made them consistentWhat changes are included in this PR?
Are these changes tested?
Yes by CI and new doc examples
Are there any user-facing changes?